Skip to content

Align prompt handler with resource pattern#2740

Merged
jlowin merged 1 commit intomainfrom
prompt-handler-alignment
Dec 26, 2025
Merged

Align prompt handler with resource pattern#2740
jlowin merged 1 commit intomainfrom
prompt-handler-alignment

Conversation

@jlowin
Copy link
Copy Markdown
Member

@jlowin jlowin commented Dec 26, 2025

Aligns the prompt handler architecture with the resource pattern established in PR #2734. Previously, task metadata extraction and MCP conversion for prompts happened in the low-level LowLevelServer.get_prompt() decorator, while resources did this work in the FastMCP handler. This inconsistency made the codebase harder to reason about.

Now both resources and prompts follow the same flow:

  1. Low-level decorator: thin wrapper, just wraps result in ServerResult
  2. FastMCP handler (_get_prompt_mcp): task metadata extraction, contextvars, MCP conversion
  3. Public API (render_prompt): middleware, provider lookup
  4. Component method (_render): returns FastMCP types (PromptResult)

This also removes several now-unused imports from low_level.py.

Move task metadata extraction and MCP conversion from
LowLevelServer.get_prompt() decorator into FastMCP._get_prompt_mcp(),
matching the resource handler architecture from PR #2734.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 26, 2025

Warning

Rate limit exceeded

@jlowin has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 27 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 61ebc64 and 6fc162d.

📒 Files selected for processing (2)
  • src/fastmcp/server/low_level.py
  • src/fastmcp/server/server.py
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch prompt-handler-alignment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@marvin-context-protocol marvin-context-protocol Bot added enhancement Improvement to existing functionality. For issues and smaller PR improvements. server Related to FastMCP server implementation or server-side functionality. labels Dec 26, 2025
@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: The integration test is timing out after 30s and hitting a 429 (Too Many Requests) rate limit from GitHub's API during teardown.

Root Cause: This failure is unrelated to the PR changes. The PR refactors prompt handler logic (moving task metadata extraction from to ), but the failing test is for GitHub MCP remote integration testing tool calls. The test is hitting GitHub Copilot's MCP endpoint () and encountering:

  1. A timeout waiting for a response to the tool call
  2. A 429 rate limit error during client disconnection/cleanup

The error occurs at line 630 in client.py during _disconnect() when waiting for the session task to complete, which then raises the 429 error that occurred during the AsyncExitStack cleanup.

Suggested Solution: This is a transient infrastructure issue with the external GitHub API, not a code problem. Options:

  1. Retry the workflow - The rate limit should reset after a brief period
  2. Add retry logic to the GitHub integration tests with exponential backoff
  3. Mock the GitHub API in these tests to avoid hitting real rate limits
  4. Increase the integration test timeout beyond 30s to account for rate limiting delays

The PR changes themselves don't affect this code path. The test should pass once GitHub's rate limits reset.

Detailed Analysis

Test Failure Log

[FAILED] tests/integration_tests/test_github_mcp_remote.py::TestGithubMCPRemote::test_call_tool_ko
Failed: Timeout (>30.0s) from pytest-timeout.

httpx.HTTPStatusError: Client error '429 Too Many Requests' for url 'https://api.githubcopilot.com/mcp/'
For more information check: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/429

What the Test Does

From test_github_mcp_remote.py:90-97:

async def test_call_tool_ko(
    self, streamable_http_client: Client[StreamableHttpTransport]
):
    """Test calling a non-existing tool"""
    async with streamable_http_client:
        assert streamable_http_client.is_connected()
        with pytest.raises(McpError, match=r"unknown tool|tool not found"):
            await streamable_http_client.call_tool("foo")

The test calls a non-existent tool "foo" on GitHub's MCP server and expects an McpError. Instead, it hangs for 30s then times out, and during cleanup gets a 429.

PR Changes

The PR moves prompt-related logic from low_level.py to server.py:

These changes only affect prompt handling, not tool calls or client transport logic.

Related Files
  • tests/integration_tests/test_github_mcp_remote.py - Integration test hitting GitHub's real API
  • src/fastmcp/client/client.py:630 - Where the 429 error surfaces during disconnect
  • src/fastmcp/server/low_level.py - Modified by PR (prompt handlers only)
  • src/fastmcp/server/server.py - Modified by PR (prompt handlers only)

@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: The integration test test_call_tool_ko is timing out after 30s and hitting a 429 (Too Many Requests) rate limit from GitHub's API during teardown.

Root Cause: This failure is unrelated to the PR changes. The PR refactors prompt handler logic (moving task metadata extraction from low_level.py to server.py), but the failing test is for GitHub MCP remote integration testing tool calls. The test is hitting GitHub Copilot's MCP endpoint (api.githubcopilot.com/mcp/) and encountering:

  1. A timeout waiting for a response to the tool call
  2. A 429 rate limit error during client disconnection/cleanup

The error occurs at line 630 in client.py during _disconnect() when waiting for the session task to complete, which then raises the 429 error that occurred during the AsyncExitStack cleanup.

Suggested Solution: This is a transient infrastructure issue with the external GitHub API, not a code problem. Options:

  1. Retry the workflow - The rate limit should reset after a brief period
  2. Add retry logic to the GitHub integration tests with exponential backoff
  3. Mock the GitHub API in these tests to avoid hitting real rate limits
  4. Increase the integration test timeout beyond 30s to account for rate limiting delays

The PR changes themselves don't affect this code path. The test should pass once GitHub's rate limits reset.

Detailed Analysis

Test Failure Log

[FAILED] tests/integration_tests/test_github_mcp_remote.py::TestGithubMCPRemote::test_call_tool_ko
Failed: Timeout (>30.0s) from pytest-timeout.

httpx.HTTPStatusError: Client error '429 Too Many Requests' for url 'https://api.githubcopilot.com/mcp/'
For more information check: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/429

What the Test Does

From `test_github_mcp_remote.py:90-97`:

async def test_call_tool_ko(
    self, streamable_http_client: Client[StreamableHttpTransport]
):
    """Test calling a non-existing tool"""
    async with streamable_http_client:
        assert streamable_http_client.is_connected()
        with pytest.raises(McpError, match=r"unknown tool|tool not found"):
            await streamable_http_client.call_tool("foo")

The test calls a non-existent tool "foo" on GitHub's MCP server and expects an `McpError`. Instead, it hangs for 30s then times out, and during cleanup gets a 429.

PR Changes

The PR moves prompt-related logic from `low_level.py` to `server.py`:

These changes only affect prompt handling, not tool calls or client transport logic.

Related Files
  • `tests/integration_tests/test_github_mcp_remote.py` - Integration test hitting GitHub's real API
  • `src/fastmcp/client/client.py:630` - Where the 429 error surfaces during disconnect
  • `src/fastmcp/server/low_level.py` - Modified by PR (prompt handlers only)
  • `src/fastmcp/server/server.py` - Modified by PR (prompt handlers only)

@jlowin jlowin merged commit 5a6f715 into main Dec 26, 2025
15 of 18 checks passed
@jlowin jlowin deleted the prompt-handler-alignment branch December 26, 2025 03:47
@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: Two integration tests for GitHub MCP remote are timing out after receiving HTTP 429 (Too Many Requests) errors from the GitHub Copilot API.

Root Cause: The tests test_call_tool_list_commits and test_call_tool_ko are hitting rate limits on the GitHub Copilot API (https://api.githubcopilot.com/mcp/). The tests wait for 30 seconds (the configured timeout) before failing, but the underlying issue is that the API returns a 429 status code indicating the rate limit has been exceeded.

Suggested Solution:

These are external integration tests that depend on third-party API availability and rate limits. Consider one of these approaches:

  1. Add retry logic with exponential backoff to the test fixture or client setup for remote API tests
  2. Skip or mark as flaky tests that depend on external rate-limited APIs in CI
  3. Mock the GitHub Copilot API for these tests instead of making real API calls
  4. Add rate limiting handling in the test suite to space out requests to external APIs

The most practical immediate fix would be option 2 - marking these specific tests to be skipped in CI or allowing them to be flaky, since they test integration with an external service that has rate limits outside of our control.

Detailed Analysis

Failed Tests:

  • tests/integration_tests/test_github_mcp_remote.py::TestGithubMCPRemote::test_call_tool_list_commits
  • tests/integration_tests/test_github_mcp_remote.py::TestGithubMCPRemote::test_call_tool_ko

Error Log Excerpt:

httpx.HTTPStatusError: Client error '429 Too Many Requests' for url 'https://api.githubcopilot.com/mcp/'
For more information check: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/429

The test starts to call the tool but the request hangs waiting for a response. After 30 seconds (the pytest timeout), the test is killed. During teardown, the client tries to disconnect and that's when the 429 error surfaces in the exception handler.

Timeline:

  1. Test calls client.call_tool()
  2. Request is sent to GitHub Copilot API
  3. API returns 429 (rate limit exceeded)
  4. Test waits up to 30s timeout
  5. Pytest timeout kills the test
  6. During cleanup, the 429 error is logged
Related Files
  • tests/integration_tests/test_github_mcp_remote.py - The failing test file
    • Line 19-22: Tests are marked with xfail if no GitHub token is present, but not for rate limiting
    • Line 99-124: test_call_tool_list_commits - Times out waiting for API response
    • Line 90-97: test_call_tool_ko - Times out waiting for API response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement to existing functionality. For issues and smaller PR improvements. server Related to FastMCP server implementation or server-side functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant